feat: Batch vote metrics (BFT-486) #157
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aakoshh
force-pushed
the
bft-486-batch-metrics
branch
from
928cf60 to
8a44e6d
Compare
pompon0
reviewed
Jul 23, 2024
pompon0
approved these changes
Jul 23, 2024
| pub(crate) last_added_vote_batch_number: vise::Gauge<u64>, | ||
|
|
||
| /// Batch number of the last batch signed by this attester. | ||
| pub(crate) last_signed_batch_number: vise::Gauge<u64>, |
There was a problem hiding this comment.
can postpone this, but can we have a histogram that would show the distribution of votes delay after an l1 batch was produced?
There was a problem hiding this comment.
Sounds interesting, I will create a ticket for this. There are a few challenges:
- the timestamp of the batch isn't available here
- currently we can receive votes earlier than we have the batch itself, so we would have to store a timestamp with the vote, and the retrospectively emit negative latencies when the batch is produced on the local node
We can have some specific workarounds:
- For example when the
last_signed_batch_numberabove changes it can act as an approximation of when the L1 batch was produced, but it's only an approximation because it depends on the polling frequency, and also it's only available on nodes which are attesters. - We could observe the time elapsed between the first and last vote. For attester nodes this would indicate the speed of propagation of votes over gossip, compared to the vote produced locally, although again the local vote might come later than the first gossiped one.
- We could implement Grzegorz's idea about only gossiping votes to nodes which have indicated they have the batch already, which would deal with the negative values; this simplifies the metrics in that we don't have to buffer timestamps and observe them the first time we learn about the batch.
- We could observe the difference between
l1_batches.created_atandl1_batches_consensus.created_atwhen we insert the QC, which would be the overall latency of gossiping votes.
There was a problem hiding this comment.
RomanBrodetski
approved these changes
Jul 24, 2024
|
Added the following metrics to the Consensus Dashboard on Grafana: 
The queries behind them are the following:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What ❔
Adds metrics around the gossiping of batch votes and the storage of batches.
Added
gossip::metrics::BATCH_VOTES_METRICS:network_gossip_batch_votes_committee_sizeto show how many attesters we have, which can be correlated with the number of votes we receive over time. Wanted to add a weight based one as well, but not sure if counting the weights monotonically would not overflow the counter.network_gossip_batch_votes_votes_addedcounts the votes received since start; with its rate-of-change we can get an idea of how many attesters are producing votesnetwork_gossip_batch_votes_weight_addedcounts the total weight of votes added since start, each normalized to [0; 1] range with the total committee weight; this way it shouldn't overflow, and its rate of growth should be 0.8-1.0 per batch.network_gossip_batch_votes_min_batch_numbershows the minimum batch we expect votes for; going up will indicate that QCs are formednetwork_gossip_batch_votes_last_added_vote_batch_numberis the number from the last vote we registered; apart from Byzantine votes it should average to go up as L1 batches are created and votes are received, even if a QC cannot be formednetwork_gossip_batch_votes_last_signed_batch_numberindicates that the current node is publishing its votes and keeping up with the L1 batchesAlso implement metrics for the
BatchStoresimilar to how theBlockStorehas them:nextforqueuedandpersistedPersistedBatchStoreoperationswait_for_...methodszksync_consensus_storage_batch_store_last_persisted_batch_qc: last batch QC savedThe latencies for some of the
BatchStoremethods could potentially be less cumbersome if we used instrumentation.Why ❔
To make the voting process observable.